Fix external js updates to multi-selects #1070
Closed
+45
−43
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I ran into some issues where updated multi selects from external javascript would frequently get out of state because the .choices variable on the Chosen object was no longer the same as the actual number of options selected. Instead of having a global variable on the chosen jquery object (which is relatively fragile because of external changes to the select element), just calculate it as needed. It's not called often and this is a fairly fast lookup for jquery and prototype, so I think this is a safe change.
|
Thanks @tjarratt I was nervous about the way selected options were being queried, so I did some testing. It turns out that looping through the raw javascript select object's options is much faster, so I moved the counting method to the abstractChosen method. Your commit has been added to this PR: #1171. Please let me know if this still solves your issue and any other thoughts you might have. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ran into some issues where updated multi selects from external
javascript (using $(select).trigger("liszt:updated") would frequently
get out of state because the #choices variable on the Chosen object
was no longer the same as the actual number of options selected.
Instead of having a global variable on the chosen jquery object
(which is relatively fragile because of external changes to the
select element), just calculate it as needed. It's not called often
and this is a fairly fast lookup for jquery and prototype, so I think
this is a safe change.
This is my first pull request on a js project using coffeescript, so
any feedback is welcome. I'm already using this patch in a production
setting, so if you'd like I could describe the actual use case that produced
this error condition, or provide a minimal reproduction of the issue but
in my opinion this is a fairly clear change that resolves some of the bugs
one would encounter using chosen with ember.js, or another UI heavy
library where updates to the select element are not always through user
interaction.